Skip to content

fix: view-only files #57335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
Koc wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: view-only files #57335

Koc wants to merge 2 commits into from
+86 −113

Conversation

@Koc
Copy link
Contributor

@Koc Koc commented Jan 5, 2026 •
edited by nickvergessen
Loading

TODO

  • Check more edge-cases (e.g. shared and own files located in nested folder)
  • Actualize tests

Checklist

@Koc Koc requested a review from a team as a code owner January 5, 2026 00:16
@Koc Koc requested review from come-nc, leftybournes, salmart-dev and yemkareems and removed request for a team January 5, 2026 00:16
@Koc Koc force-pushed the bugfix/prevent-download-view-ony-files branch from f5ab743 to fc61b22 Compare January 5, 2026 00:34
come-nc
come-nc reviewed Jan 5, 2026
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not understand the PR, it checks the same files twice, once as paths, once as nodes?
Why is that needed? Why was it not working before?

@Koc Koc force-pushed the bugfix/prevent-download-view-ony-files branch 2 times, most recently from 975782e to 17b40e1 Compare January 6, 2026 11:07
@Koc
Copy link
Contributor Author

Koc commented Jan 6, 2026

@come-nc

Why was it not working before?

Good question, but I don't know. For some reason we're still able to download non-downloadable files using workarounds that I explained in the PR description. I've double-tested it on master branch before PR opening.

Regarding duplicated checks - I've added one more commit that simplifies this. So, now we recursively collecting files to download only once and perform check only once. Also in previous implementation non-downloaded files break whole download (check not works, but idea was the same). Now we're just skipping such files and not break download process.

@Koc Koc force-pushed the bugfix/prevent-download-view-ony-files branch 2 times, most recently from 13673e9 to 26992d1 Compare January 6, 2026 11:20
@Koc Koc force-pushed the bugfix/prevent-download-view-ony-files branch 2 times, most recently from f7b455d to 34a4eda Compare January 14, 2026 17:11
@Koc Koc force-pushed the bugfix/prevent-download-view-ony-files branch from 34a4eda to ed1a86a Compare January 14, 2026 17:31
@Koc
Copy link
Contributor Author

Koc commented Jan 14, 2026

@susnux @AndyScherzinger guys, can you take a look on it, please?

@nickvergessen nickvergessen changed the title fix: Prevent download of view-only files fix: view-only files Jan 14, 2026
@AndyScherzinger
Copy link
Member

AndyScherzinger commented Jan 14, 2026

@Koc I pinge @sorbaugh to have engineering reviewing the PR. I am not a php/js dev unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@come-nc come-nc come-nc left review comments

@salmart-dev salmart-dev Awaiting requested review from salmart-dev salmart-dev is a code owner automatically assigned from nextcloud/server-backend

@yemkareems yemkareems Awaiting requested review from yemkareems yemkareems is a code owner automatically assigned from nextcloud/server-backend

@leftybournes leftybournes Awaiting requested review from leftybournes leftybournes is a code owner automatically assigned from nextcloud/server-backend

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Koc @AndyScherzinger @come-nc